[lldb][RISCV] Implement trap handler unwind plan #166531

sga-sc · 2025-11-05T11:02:26Z

This patch introduces special unwind plan for trap handling for RISC-V and fixes TestHandleAbort

llvmbot · 2025-11-05T11:02:56Z

@llvm/pr-subscribers-lldb

@llvm/pr-subscribers-backend-risc-v

Author: Georgiy Samoylov (sga-sc)

Changes

This patch introduces special unwind plan for trap handling for RISC-V

Full diff: https://github.com/llvm/llvm-project/pull/166531.diff

2 Files Affected:

(modified) lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp (+4)
(modified) lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp (+48-1)

diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
index a5547a4699ca9..d209980d65589 100644
--- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
+++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
@@ -735,6 +735,8 @@ UnwindPlanSP ABISysV_riscv::CreateFunctionEntryUnwindPlan() {
   plan_sp->AppendRow(std::move(row));
   plan_sp->SetSourceName("riscv function-entry unwind plan");
   plan_sp->SetSourcedFromCompiler(eLazyBoolNo);
+  plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolNo);
+
   return plan_sp;
 }
 
@@ -761,6 +763,8 @@ UnwindPlanSP ABISysV_riscv::CreateDefaultUnwindPlan() {
   plan_sp->SetSourceName("riscv default unwind plan");
   plan_sp->SetSourcedFromCompiler(eLazyBoolNo);
   plan_sp->SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolNo);
+
   return plan_sp;
 }
 
diff --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
index da14da44f5939..d7c15f19f5d69 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
@@ -15,6 +15,7 @@
 #endif
 
 #include "Plugins/Process/Utility/LinuxSignals.h"
+#include "Plugins/Process/Utility/lldb-riscv-register-enums.h"
 #include "Utility/ARM64_DWARF_Registers.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
@@ -220,6 +221,7 @@ void PlatformLinux::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
   m_trap_handlers.push_back(ConstString("__kernel_rt_sigreturn"));
   m_trap_handlers.push_back(ConstString("__restore_rt"));
+  m_trap_handlers.push_back(ConstString("__vdso_rt_sigreturn"));
 }
 
 static lldb::UnwindPlanSP GetAArch64TrapHandlerUnwindPlan(ConstString name) {
@@ -302,12 +304,57 @@ static lldb::UnwindPlanSP GetAArch64TrapHandlerUnwindPlan(ConstString name) {
   return unwind_plan_sp;
 }
 
+static lldb::UnwindPlanSP GetRISCVTrapHandlerUnwindPlan(ConstString name) {
+  if (name != "__vdso_rt_sigreturn")
+    return UnwindPlanSP{};
+
+  UnwindPlan::RowSP row = std::make_shared<UnwindPlan::Row>();
+  row->SetOffset(0);
+
+  // In the signal trampoline frame, sp points to an rt_sigframe[1], which is:
+  //  - 128-byte siginfo struct
+  //  - ucontext struct:
+  //     - 8-byte long (uc_flags)
+  //     - 8-byte pointer (*uc_link)
+  //     - 24-byte struct (uc_stack)
+  //     - 8-byte struct (uc_sigmask)
+  //     - 120-byte of padding to allow sigset_t to be expanded in the future
+  //     - 8 bytes of padding because sigcontext has 16-byte alignment
+  //     - struct sigcontext uc_mcontext
+  // [1]
+  // https://github.com/torvalds/linux/blob/master/arch/riscv/kernel/signal.c
+
+  constexpr size_t siginfo_size = 128;
+  constexpr size_t uc_flags_size = 8;
+  constexpr size_t uc_link_ptr_size = 8;
+  constexpr size_t uc_stack_size = 24;
+  constexpr size_t uc_sigmask_size = 8;
+  constexpr size_t padding_size = 128;
+
+  constexpr size_t offset = siginfo_size + uc_flags_size + uc_link_ptr_size +
+                            uc_stack_size + uc_sigmask_size + padding_size;
+
+  row->GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_riscv, offset);
+  for (uint32_t reg_num = 0; reg_num <= 31; ++reg_num)
+    row->SetRegisterLocationToAtCFAPlusOffset(reg_num, reg_num * 8, false);
+
+  UnwindPlanSP unwind_plan_sp = std::make_shared<UnwindPlan>(eRegisterKindLLDB);
+  unwind_plan_sp->AppendRow(row);
+  unwind_plan_sp->SetSourceName("RISC-V Linux sigcontext");
+  unwind_plan_sp->SetSourcedFromCompiler(eLazyBoolYes);
+  unwind_plan_sp->SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolYes);
+
+  return unwind_plan_sp;
+}
+
 lldb::UnwindPlanSP
 PlatformLinux::GetTrapHandlerUnwindPlan(const llvm::Triple &triple,
                                         ConstString name) {
   if (triple.isAArch64())
     return GetAArch64TrapHandlerUnwindPlan(name);
-
+  if (triple.isRISCV())
+    return GetRISCVTrapHandlerUnwindPlan(name);
   return {};
 }

DavidSpickett · 2025-11-05T11:35:20Z

Unless this fixes an existing test case, you could adapt the one from 9db2541 to work for RISC-V.

I put it in the AArch64 folder but it can be moved somewhere generic. The specific bits are the x register naming and the undefined encoding.

sga-sc · 2025-11-05T11:57:32Z

I forgot to mention that this patch fixes TestHandleAbort on RISC-V. I'll change PR description for this

DavidSpickett · 2025-11-05T13:24:56Z

Cool. Now I see that I enabled that test for AArch64 Linux at the time, but added the new one to check the register values. Not sure why I didn't hack something into the existing one.

Anyway, this is a net improvement as is and if you want to add more checks later there's something for you to crib from.

sga-sc · 2025-11-12T12:11:58Z

@clayborg @jasonmolenda @lenary @mgorny could you take a look please?

lenary · 2025-11-13T21:56:13Z

lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp

+
+  row.GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_riscv, offset);
+  for (uint32_t reg_num = 0; reg_num <= 31; ++reg_num)
+    row.SetRegisterLocationToAtCFAPlusOffset(reg_num, reg_num * 8, false);


Do we need a row to say where pc is stored? It's not part of regs 0-31, as far as I can tell.

No, pc register is a part of regs 0-31:

llvm-project/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h

Line 19 in 80ae168

gpr_pc_riscv = gpr_first_riscv,

Ah, interesting.

Should we be pointing to the FP regs in uc_mcontext too?

Thank you for your comment, I added FP regs recovery in this plan

lenary · 2025-11-14T22:12:31Z

lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp

+  UnwindPlanSP unwind_plan_sp = std::make_shared<UnwindPlan>(eRegisterKindLLDB);
+  unwind_plan_sp->AppendRow(std::move(row));
+  unwind_plan_sp->SetSourceName("RISC-V Linux sigcontext");
+  unwind_plan_sp->SetSourcedFromCompiler(eLazyBoolYes);


Both this and the AArch64 equivalent (written by @DavidSpickett) seem to set this to Yes. Is this right? This doesn't seem compiler-written to me, but I don't know what the intention behind this setting is - I'm sure David does.

Yeah you'd like to think that but I don't know why I set it that way (https://reviews.llvm.org/D112069). Possibly because the code that would try to use the unwind plan just wouldn't unless it was set.

So that's one way to justify it here, if you change it, does it still get used?

@jasonmolenda added this back in 60f0bd4. Maybe he remembers what it means.

I see things setting it to no even when they are plans built inside LLDB. Perhaps the reason we set it to yes in a trap handler unwind plan is because the plan is representing a compiler generated thing?

Whereas ABI plugins do not represent compiler decisions. Beyond "I should follow the ABI" of course.

(IIRC the AArch64 trap handler actually has CFI directives commented out due to some crash somewhere else if they were actually used)

I changed this flag to eLazyBoolNo, because this unwind plan wasn't emitted by the compiler. In this case linux tells us how to do unwinding.

Also unwinding works correctly both with eLazyBoolNo and eLazyBoolYes value of this flag

lenary

I am happy with this, but please wait for @DavidSpickett or another LLDB maintainer to re-review, as I am quite far from being an LLDB expert.

JDevlieghere · 2025-11-17T22:06:18Z

lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp

+static lldb::UnwindPlanSP GetRISCVTrapHandlerUnwindPlan(ConstString name,
+                                                        uint32_t fp_flags) {
+  if (name != "__vdso_rt_sigreturn")
+    return UnwindPlanSP{};


Suggested change

return UnwindPlanSP{};

return{};

JDevlieghere · 2025-11-17T22:06:55Z

lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp

+                                               false);
+    }
+
+    // CSR for FP registers always has 32-bit length


Suggested change

// CSR for FP registers always has 32-bit length

// CSR for FP registers always has 32-bit length.

JDevlieghere · 2025-11-17T22:07:33Z

lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp

+    fpr_size = 16;
+    break;
+  default:
+    llvm_unreachable("Invalid RISC-V FP flags");


These fp_flags are coming from the ABI right, they're not user input (i.e. nobody can craft a binary that would have an invalid flag here?)

@JDevlieghere take a look, please

github-actions · 2025-11-18T12:15:27Z

🐧 Linux x64 Test Results

33167 tests passed
490 tests skipped

lenary · 2025-11-19T07:02:09Z

lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp

+  default:
+    break;


This is actually unreachable. The FP Flags were masked by a 2-bit mask, so there are only 4 possible values, which are all accounted for in the switch.

…p handling After we introduced a special unwind plan for trap handling, we should mark that other unwind plans for RISC-V can't be used in the same case.

sga-sc requested a review from JDevlieghere as a code owner November 5, 2025 11:02

llvmbot added lldb backend:RISC-V labels Nov 5, 2025

sga-sc force-pushed the users/sga-sc/lldb_handle_abort branch from 3b72ba2 to f52f1e7 Compare November 5, 2025 12:42

DavidSpickett requested review from clayborg and jasonmolenda November 5, 2025 13:25

lenary reviewed Nov 13, 2025

View reviewed changes

lenary reviewed Nov 14, 2025

View reviewed changes

sga-sc force-pushed the users/sga-sc/lldb_handle_abort branch 2 times, most recently from a882f51 to ce07952 Compare November 17, 2025 14:10

lenary approved these changes Nov 17, 2025

View reviewed changes

JDevlieghere reviewed Nov 17, 2025

View reviewed changes

sga-sc force-pushed the users/sga-sc/lldb_handle_abort branch from ce07952 to 9944a02 Compare November 18, 2025 12:08

sga-sc force-pushed the users/sga-sc/lldb_handle_abort branch from 9944a02 to 4aa2ed1 Compare November 18, 2025 12:19

lenary reviewed Nov 19, 2025

View reviewed changes

sga-sc added 3 commits November 19, 2025 12:40

[lldb][RISCV] Implement trap handler unwind plan

5cf5867

[lldb][RISCV] Mark default unwind plans they couldn't be used for tra…

f20b5de

…p handling After we introduced a special unwind plan for trap handling, we should mark that other unwind plans for RISC-V can't be used in the same case.

[lldb] Change GetTrapHandlerUnwindPlan interface

1747631

sga-sc force-pushed the users/sga-sc/lldb_handle_abort branch from 4aa2ed1 to 1747631 Compare November 19, 2025 09:40

	// CSR for FP registers always has 32-bit length
	// CSR for FP registers always has 32-bit length.

[lldb][RISCV] Implement trap handler unwind plan #166531

Are you sure you want to change the base?

[lldb][RISCV] Implement trap handler unwind plan #166531

Conversation

sga-sc commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DavidSpickett commented Nov 5, 2025

Uh oh!

sga-sc commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DavidSpickett commented Nov 5, 2025

Uh oh!

sga-sc commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lenary left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sga-sc Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sga-sc commented Nov 5, 2025 •

edited

Loading

llvmbot commented Nov 5, 2025 •

edited

Loading

sga-sc commented Nov 5, 2025 •

edited

Loading

sga-sc Nov 18, 2025 •

edited

Loading

github-actions bot commented Nov 18, 2025 •

edited

Loading